Fixes the rendering logic inside the environment classes #515

Dhoeller19 · 2024-06-14T08:32:32Z

Description

Fixes #476

The parameter substeps in the Simulation config had no influence on the rendering frequency.
Additionally, app.update was not properly called when initializing the stage due to the render method overload. This led to problems when modifying the render interval.

Merged physics stepping and rendering inside the step method in the simulation config.
Renamed substeps to render_interval in the simulation config.
Updated all the tasks

Type of change

Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

I have run the pre-commit checks with ./isaaclab.sh --format
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have run all the tests with ./isaaclab.sh --test and they pass
I have updated the changelog and the corresponding version in the extension's config/extension.toml file
I have added my name to the CONTRIBUTORS.md or my name already exists there

source/extensions/omni.isaac.lab/omni/isaac/lab/sim/simulation_context.py

source/extensions/omni.isaac.lab/docs/CHANGELOG.rst

source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py

source/standalone/workflows/sb3/train.py

source/standalone/workflows/rsl_rl/train.py

source/standalone/workflows/rl_games/train.py

…ased_env.py Co-authored-by: Mayank Mittal <[email protected]> Signed-off-by: David Hoeller <[email protected]>

Co-authored-by: Mayank Mittal <[email protected]> Signed-off-by: David Hoeller <[email protected]>

…nto fix/render_interval

Batou1406 · 2024-06-28T15:22:22Z

Possible Bug

I noticed some weird behaviours using the render_interval. A policy able to walk perfectly for render_interval=1, was not able to walk decently for other value of render_interval...

I just made the upgrade from Orbit (and Isaac Sim 2023.1.1) to IsaacLab (and Isaac Sim 4.0) this week, so I still may be doing something wrong, but this feature was definitely not working as intended.

Possible Fix

After looking to the commits, I saw a change in the logic. Previously self.sim.step(render=False) was called always with render=False and then a call to self.sim.render() (outside the decimation for loop) was made if necessary. The update changed the logic and a single call to self.sim.step(render=render) was made with render a variable that determines if a rendering is necessary.

I noticed that even though these two approach should be equivalent, the simulation did not behave the same. In one case the robot would be able to walk, in the other not.

In order to fix the behaviour, I had to change the operation flow from :

for _ in range(self.cfg.decimation):
    self._sim_step_counter += 1
    # set actions into buffers
    self.action_manager.apply_action()
    # set actions into simulator
    self.scene.write_data_to_sim()
    render = self._sim_step_counter % self.cfg.sim.render_interval == 0 and (
        self.sim.has_gui() or self.sim.has_rtx_sensors()
    )
    # simulate
    self.sim.step(render=render)
    # update buffers at sim dt
    self.scene.update(dt=self.physics_dt)

to

for _ in range(self.cfg.decimation):
    self._sim_step_counter += 1
    # set actions into buffers
    self.action_manager.apply_action()
    # set actions into simulator
    self.scene.write_data_to_sim()
    # simulate
    self.sim.step(render=False)
    # update buffers at sim dt
    self.scene.update(dt=self.physics_dt)
    # render
    if self._sim_step_counter % self.cfg.sim.render_interval == 0 and (self.sim.has_gui() or self.sim.has_rtx_sensors()):
        self.sim.render()

This resolve the behaviour and my policy was able to walk perfectly no matter the render_interval.

I can't explain why the first approach is not working but it seems to me that self.sim.step(render=True) is not behaving as I would expect.

I hope this help, and if I was doing something wrong, I'd be happy to be corrected if I made a mistake.

Dhoeller19 · 2024-07-01T19:46:14Z

@Batou1406 this is a great catch thanks a lot.
The fix is in the pipeline and will be merged soon: #614

Fixes isaac-sim#476 The parameter substeps in the Simulation config had no influence on the rendering frequency. Additionally, app.update was not properly called when initializing the stage due to the render method overload. This led to problems when modifying the render interval. - Merged physics stepping and rendering inside the step method in the simulation config. - Renamed substeps to render_interval in the simulation config. - Updated all the tasks ## Type of change - Breaking change (fix or feature that would cause existing functionality to not work as expected) ## Checklist - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have run all the tests with `./isaaclab.sh --test` and they pass - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there

# Description  Adds a cleanup of remaining async events in the Mimic data generation code. The cleanup cancels remaining events after the environment is closed and prior to sim app shutdown. This prevents left over events from trying to use the env after it is closed. ## Type of change  - Bug fix (non-breaking change which fixes an issue) ## Checklist - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there

David Hoeller added 8 commits June 11, 2024 10:35

Fixed render interval

10c045d

Update docstring

07823c0

Trigger Build

a37e95a

Propagate removal of substeps

8ba0556

Merge branch 'main' into fix/render_interval

00c65bb

Improve changelog

6e0530c

Merge branch 'main' into fix/render_interval

e474d4d

Formatting

1fe46eb

Dhoeller19 requested a review from kellyguo11 June 14, 2024 09:24

kellyguo11 approved these changes Jun 14, 2024

View reviewed changes